[TrimmableTypeMap] Root manifest-referenced types as unconditional#11037
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the TrimmableTypeMap pipeline so Android components referenced only from a user-provided AndroidManifest.xml template are treated as unconditionally preserved, preventing them from being trimmed when there are no direct managed references.
Changes:
- Parse the manifest template for component declarations and mark matching scanned
JavaPeerInfoentries asIsUnconditional. - Integrate trimmable typemap generation more deeply into the MSBuild pipeline (targets + runtime host config), including generation of native typemap stub
.llsources. - Expand scanning/generator models for manifest-related metadata and update tests for the updated JNI/native-callback naming behavior.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/OverrideDetectionTests.cs | Updates expected derived native callback name for override detection. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs | Adjusts expectations for generated wrapper surface. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs | Adjusts expectations for generated wrapper surface. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Adds direct generator tests, including manifest rooting scenarios. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs | Updates JNI name conversion and generated native method naming expectations. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateTrimmableTypeMapTests.cs | Refactors MSBuild task tests to the new adapter behavior and outputs. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Makes the task a thin adapter over TrimmableTypeMapGenerator and passes manifest/acw-map parameters. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs | Allows missing JNIEnvInit tokens in the CoreCLR/trimmable path by using token 0. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateEmptyTypemapStub.cs | New task to generate empty LLVM IR typemap stubs per ABI. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Wires typemap generation target, JCW copying, manifest/acw-map handling, and native stub generation. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets | Hooks generated TypeMap assemblies into ILLink and assembly store for CoreCLR builds. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Adds runtime feature flag defaulting trimmable typemap off for NativeAOT. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.MonoVM.targets | Adds runtime feature flag defaulting trimmable typemap off for MonoVM. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/ApplicationRegistration.Trimmable.java | Adds a trimmable-path ApplicationRegistration stub with empty registration. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/PreserveLists/Trimmable.CoreCLR.xml | Adds an ILLink descriptor to preserve Android.Runtime.JNIEnvInit.Initialize. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapTypes.cs | Adds shared result/config records (TrimmableTypeMapResult, ManifestConfig). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Implements manifest-rooting + orchestrates scan → typemap → JCW → manifest/acw-map. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesPermissionInfo.cs | New manifest model record for assembly-level uses-permission. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesLibraryInfo.cs | New manifest model record for assembly-level uses-library. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesFeatureInfo.cs | New manifest model record for assembly-level uses-feature. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesConfigurationInfo.cs | New manifest model record for assembly-level uses-configuration. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PropertyInfo.cs | New manifest model record for assembly-level property entries. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionTreeInfo.cs | New manifest model record for permission-tree. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionInfo.cs | New manifest model record for permission. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionGroupInfo.cs | New manifest model record for permission-group. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/MetaDataInfo.cs | New/updated model for metadata entries (type + assembly level). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Captures component attribute data + improves native callback name derivation and declaring type parsing. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Makes IsUnconditional mutable post-scan; documents component attribute storage. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/IntentFilterInfo.cs | New model for intent-filter data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ComponentInfo.cs | New model for component attributes and related intent filters/metadata. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyManifestInfo.cs | New model to aggregate assembly-level manifest attributes across assemblies. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs | Adds decoding/parsing for assembly-level manifest attributes + richer type attribute capture. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/NullableExtensions.cs | Adds NRT-friendly string helper extensions for netstandard2.0. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestModel.cs | Removes the old manifest model types (superseded by Scanner/ record types). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs | Refactors manifest generation to accept a pre-loaded template document. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JniSignatureHelper.cs | Fixes JNI name → Java name conversion for nested types ($ → .) in Java source contexts. |
|
|
||
| // Write merged acw-map.txt if requested | ||
| if (!acwMapOutputPath.IsNullOrEmpty ()) { | ||
| Directory.CreateDirectory (Path.GetDirectoryName (acwMapOutputPath)); |
There was a problem hiding this comment.
Directory.CreateDirectory (Path.GetDirectoryName (acwMapOutputPath)) can throw because Path.GetDirectoryName() returns null when acwMapOutputPath has no directory component (e.g., just acw-map.txt). Guard against null (or use the current directory) before calling CreateDirectory.
| Directory.CreateDirectory (Path.GetDirectoryName (acwMapOutputPath)); | |
| var acwDirectory = Path.GetDirectoryName (acwMapOutputPath); | |
| if (!string.IsNullOrEmpty (acwDirectory)) { | |
| Directory.CreateDirectory (acwDirectory); | |
| } |
| var peersByDotName = new Dictionary<string, List<JavaPeerInfo>> (StringComparer.Ordinal); | ||
| foreach (var peer in allPeers) { | ||
| var dotName = peer.JavaName.Replace ('/', '.').Replace ('$', '.'); | ||
| if (!peersByDotName.TryGetValue (dotName, out var list)) { | ||
| list = []; | ||
| peersByDotName [dotName] = list; | ||
| } | ||
| list.Add (peer); | ||
| } |
There was a problem hiding this comment.
RootManifestReferencedTypes builds its lookup key by converting peer.JavaName with .Replace ('$', '.'), but Android manifest class names for nested types use $ (e.g., com.example.Outer$Inner), so those will never match and won't be rooted. Also, manifest android:name commonly supports relative names (e.g., .MyActivity or MyActivity), which won't match the current dictionary keys either. Normalize android:name values (resolve relative names using the manifest package attribute) and avoid converting $ to . for manifest matching.
| Assert.Throws<ArgumentNullException> (() => generator.Execute ( | ||
| null!, Path.Combine (testDir, "out"), Path.Combine (testDir, "java"), | ||
| new Version (11, 0), new HashSet<string> ())); |
There was a problem hiding this comment.
This test uses the null-forgiving operator (null!) to pass a null value. Repository guidance prefers avoiding ! even in tests; consider using a nullable local variable (or an explicit cast) so the test still passes null without the null-forgiving operator.
| <Target Name="_PrepareNativeAssemblySources" | ||
| Condition=" '$(_AndroidRuntime)' != 'NativeAOT' " | ||
| Inputs="@(_BuildTargetAbis)" | ||
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemaps.%(Identity).ll')"> |
There was a problem hiding this comment.
_PrepareNativeAssemblySources declares its Outputs as typemaps.%(Identity).ll, but GenerateEmptyTypemapStub actually writes typemap.{abi}.ll (no trailing 's'). This mismatch breaks MSBuild incremental tracking for the target and can cause it to rerun unnecessarily or appear out-of-date forever. Update the Outputs transform to match the generated file name(s).
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemaps.%(Identity).ll')"> | |
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemap.%(Identity).ll')"> |
| <Target Name="_GenerateTrimmableTypeMap" | ||
| Condition=" '$(_AndroidTypeMapImplementation)' == 'trimmable' and '@(ReferencePath->Count())' != '0' " | ||
| AfterTargets="CoreCompile" | ||
| Inputs="@(ReferencePath);$(IntermediateOutputPath)$(TargetFileName);$(_AndroidManifestAbs)" | ||
| Outputs="$(_TypeMapOutputDirectory)$(_TypeMapAssemblyName).dll"> | ||
|
|
||
| <ItemGroup> | ||
| <_TypeMapInputAssemblies Include="@(ReferencePath)" /> | ||
| <_TypeMapInputAssemblies Include="@(ResolvedAssemblies)" /> | ||
| <_TypeMapInputAssemblies Include="@(ResolvedFrameworkAssemblies)" /> | ||
| <_TypeMapInputAssemblies Include="$(IntermediateOutputPath)$(TargetFileName)" | ||
| Condition="Exists('$(IntermediateOutputPath)$(TargetFileName)')" /> | ||
| </ItemGroup> | ||
|
|
||
| <GenerateTrimmableTypeMap | ||
| ResolvedAssemblies="@(_ResolvedAssemblies)" | ||
| ResolvedAssemblies="@(_TypeMapInputAssemblies)" | ||
| OutputDirectory="$(_TypeMapOutputDirectory)" | ||
| JavaSourceOutputDirectory="$(_TypeMapJavaOutputDirectory)" | ||
| AcwMapDirectory="$(_PerAssemblyAcwMapDirectory)" | ||
| TargetFrameworkVersion="$(TargetFrameworkVersion)"> | ||
| AcwMapDirectory="$(_TypeMapBaseOutputDir)acw-maps/" | ||
| TargetFrameworkVersion="$(TargetFrameworkVersion)" | ||
| ManifestTemplate="$(_AndroidManifestAbs)" | ||
| MergedAndroidManifestOutput="$(_TypeMapBaseOutputDir)AndroidManifest.xml" | ||
| PackageName="$(_AndroidPackage)" | ||
| ApplicationLabel="$(_ApplicationLabel)" | ||
| VersionCode="$(_AndroidVersionCode)" | ||
| VersionName="$(_AndroidVersionName)" | ||
| AndroidApiLevel="$(_AndroidApiLevel)" | ||
| SupportedOSPlatformVersion="$(SupportedOSPlatformVersion)" | ||
| AndroidRuntime="$(_AndroidRuntime)" | ||
| Debug="$(AndroidIncludeDebugSymbols)" | ||
| NeedsInternet="$(AndroidNeedsInternetPermission)" | ||
| EmbedAssemblies="$(EmbedAssembliesIntoApk)" | ||
| ManifestPlaceholders="$(AndroidManifestPlaceholders)" | ||
| CheckedBuild="$(_AndroidCheckedBuild)" | ||
| ApplicationJavaClass="$(AndroidApplicationJavaClass)" | ||
| AcwMapOutputFile="$(IntermediateOutputPath)acw-map.txt"> |
There was a problem hiding this comment.
_GenerateTrimmableTypeMap has an Inputs/Outputs incremental definition, but the task output is also affected by many property values passed to GenerateTrimmableTypeMap (e.g., _ApplicationLabel, _AndroidVersionCode, AndroidNeedsInternetPermission, AndroidManifestPlaceholders, etc.). If any of these change without changing @(ReferencePath)/manifest template, MSBuild can skip the target and leave a stale merged manifest/acw-map in the intermediate output. Consider including these properties in Inputs (or removing the Inputs/Outputs incremental gating / splitting manifest generation into its own target with correct inputs).
db8fc87 to
6500aa5
Compare
590d41d to
ff7258a
Compare
Parse the user's AndroidManifest.xml template for activity, service, receiver, and provider elements with android:name attributes. Mark matching scanned Java peer types as IsUnconditional = true so the ILLink TypeMap step preserves them even if no managed code references them directly. Changes: - JavaPeerInfo.IsUnconditional: init → set (must be mutated after scanning) - TrimmableTypeMapGenerator: add warn callback, RootManifestReferencedTypes() called between scanning and typemap generation - GenerateTrimmableTypeMap task: pass Log.LogWarning as warn callback - 4 new xUnit tests covering rooting, unresolved warnings, already-unconditional skip, and empty manifest Replaces #11016 (closed — depended on old PR shape with TaskLoggingHelper). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix RootManifestReferencedTypes to resolve relative android:name values (.MyActivity, MyActivity) using manifest package attribute - Keep $ separator in peer lookup keys so nested types (Outer$Inner) match correctly against manifest class names - Guard Path.GetDirectoryName against null return for acw-map path - Fix pre-existing compilation error: load XDocument from template path before passing to ManifestGenerator.Generate - Add tests for relative name resolution and nested type matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e1819e1 to
c504fa1
Compare
Summary
Parses the user's AndroidManifest.xml template for
activity,service,receiver, andproviderelements withandroid:nameattributes. Marks matching scanned Java peer types asIsUnconditional = trueso the ILLink TypeMap step preserves them even if no managed code references them directly.Replaces #11016 (closed — depended on old PR shape).
Changes (4 files, +197/-3)
JavaPeerInfo.IsUnconditional:init→set(must be mutated after scanning when the manifest references a type)TrimmableTypeMapGenerator:Action<string>? warncallback for warning-level messagesRootManifestReferencedTypes(allPeers, manifestTemplatePath)— loads and parses the manifest XMLRootManifestReferencedTypes(allPeers, XDocument)—internalfor direct testing; builds a lookup by Java dot-name, then marks matching peers as unconditionalExecute()GenerateTrimmableTypeMaptask: passesmsg => Log.LogWarning(msg)as the warn callbackRootManifestReferencedTypes_RootsMatchingPeers— verifies matching type gets rootedRootManifestReferencedTypes_WarnsForUnresolvedTypes— verifies warning for unknown typesRootManifestReferencedTypes_SkipsAlreadyUnconditional— no-op for already-rootedRootManifestReferencedTypes_EmptyManifest_NoChanges— empty manifest doesn't affect peersDependencies
Stacked on #11036 (PR 4: build pipeline), which includes #11032, #11033, #11034, #11035.